Skip to content

Conversation

@cyppe
Copy link
Contributor

@cyppe cyppe commented Jun 20, 2025

Here is the additional PoC for adding a simple cache for static rules as extended logic to PR #1043

Please share feedback as it's potentially a bit "hardcoded" with a list of unsafe rules to cache, maybe there is some better approach.

But the positive thing is that using the same benchmark as in PR #1043 it saves additionally 20% (around 200ms in time instead of about 258ms.

@rubenvanassche
Copy link
Member

Hi @cyppe,

Thanks for the PR I'm not really sure if it is already time to add in such layer of caching before we find out the details. Atm caching isn't possible for a lot of rules, that's okay. But I think we want to make it possible for nested data classes/collections since you'll probably have a lot of them. So without support for that it feels like adding a lot of code we need to maintain without a lot of extra gains.

So that open up the question is it possible to do this for data classes/collections and extended even morphable properties(which always are dataclasses for now btw). I think so but we need to rewrite your PR differently. Atm we're calculating the rules when validating but why not do this in advance? We've got a system with DataProperties and Dataclasses in place which can be completely cached once deploying. So wouldn't it be possible to cache rules on the DataClass (heck even the DataProperty if that makes things easier?).

Then we could also solve the issue with the morphable properties since it will always be a data object and thus we'll probably have the rules available somewhere.

As an added benefit, since we would cache everything nothing stops us from using cached rules for individual data objects not in a collection. That should make things even faster.

As for the $potentiallyUnsafeAttributes let's not do it like that but introduce an interface UncachableValidationAttribute and tag all attributes which arent't cacheable. Easier to read and maintain.

This would be a really cool feature but to do it good it will require a bit of work. Feel free to try it out, otherwise it something on my list to once take a look at but not sure when 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants